fix(workers): pass windowsHide to inner CLI spawns so no console window pops on Windows#256
fix(workers): pass windowsHide to inner CLI spawns so no console window pops on Windows#256efenocchi wants to merge 5 commits into
Conversation
…ow pops on Windows The wiki worker and skillify gate worker are spawned detached and console-less (spawn-detached.ts already sets windowsHide on the worker itself). But the INNER execFileSync that runs the summarizer CLI (claude -p / codex exec / cursor --print / pi --print / hermes -z) did not pass windowsHide, so Windows allocated a fresh visible console window titled after the CLI exe — users reported a bare "claude.exe" window appearing at session end. Add windowsHide: true (CREATE_NO_WINDOW, no-op on POSIX) to: - both branches of buildClaudeInvocation / buildTrailingPromptInvocation (covers the claude/codex/cursor/pi wiki workers) - the hermes wiki worker's direct execFileSync - the skillify gate-runner's CLI spawn Tests: unit assertions on every builder branch, plus source-level guards for the hermes worker and gate-runner (their dispatch tests exec a real binary and cannot observe the options object).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesWindows console suppression across worker spawns
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Coverage ReportScope: files changed in this PR. Enforced threshold: 90% per metric (per file via
File Coverage — 9 files changed
Generated for commit 6e98012. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/claude-code/skillify-gate-runner.test.ts`:
- Around line 12-13: The test currently only checks for the substring
"windowsHide: true" which can be false-positive; update the assertion to scope
the check to the actual spawn call by either (a) using a focused regex that
matches the call structure (e.g. a regex that finds execFileSync(...) or
runChildProcess(...) whose options object contains windowsHide:\s*true) or (b)
parsing the file with an AST parser (e.g. `@babel/parser`) and asserting that a
CallExpression with callee name execFileSync or runChildProcess has an options
ObjectExpression containing a Property with key "windowsHide" and value true;
replace the expect(src).toContain(...) with the scoped regex or AST-based
assertion so the test only passes when windowsHide is set inside the actual
spawn call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 086f8883-4f93-4e3b-b11f-cc945b42c2a7
📒 Files selected for processing (6)
src/hooks/hermes/wiki-worker.tssrc/hooks/wiki-worker-spawn.tssrc/skillify/gate-runner.tstests/claude-code/skillify-gate-runner.test.tstests/claude-code/wiki-worker-windows.test.tstests/hermes/hermes-wiki-worker-source.test.ts
Substring toContain could false-positive if the token appeared anywhere in the file. Tie the assertion to the runChildProcess / execFileSync call structure ([^)]* = no closing paren between call open and the option) per CodeRabbit review.
Resolve the tests/claude-code/skillify-gate-runner.test.ts conflict by keeping main's new POSIX runGate-spawn suite and buildArgs import alongside this branch's windowsHide source guard (merged the readFileSync import into main's fs import list). Re-apply windowsHide: true to the inner summarizer-CLI spawn sites that main added after this PR was written, so a detached, console-less worker never pops a visible console window on Windows (CREATE_NO_WINDOW; no-op on POSIX): - src/hooks/wiki-worker-spawn.ts buildStdinPromptInvocation (doc refresh/generate path, execFileSync'd by refresh-llm.ts runHostPrompt) - src/skillify/stage-memory.ts (threaded through planClaudeSpawn into the spawn call) - src/skillify/advisor.ts (mine-local advisor gate claude spawn) - src/skillify/claude-model.ts (skillopt judge/proposer claude spawn) - src/commands/mine-local.ts (mine-local summarizer claude -p spawn) - src/hooks/commit-kpi-extract.ts (detached claude/codex kpi spawn) The claude/codex/cursor/pi wiki-workers and the hermes worker + gate-runner already inherit windowsHide from the merged builders / their own patched spawns.
Assert buildStdinPromptInvocation / buildClaudeStdinInvocation (main's doc refresh/generate builders) carry windowsHide on both the .cmd shell branch and the .exe/Unix branch — the same unit-level guard the other two builders already have. Add source-level guards (tests/claude-code/inner-cli-spawn-windowshide-source.test.ts) for the spawn sites that exec a real summarizer binary and so cannot observe the options object in a unit test: mine-local, advisor, claude-model, commit-kpi-extract, and stage-memory's plan threading. Each regex is scoped to the specific spawn/exec call so it fails if the option drifts out of that spawn.
Problem
Users reported a bare
claude.execonsole window flashing up at session end on Windows.The wiki worker and skillify gate worker are themselves spawned detached and console-less (
spawn-detached.tsalready setswindowsHideon the worker process). But the innerexecFileSyncthat runs the summarizer CLI (claude -p/codex exec/cursor --print/pi --print/hermes -z) did not passwindowsHide, so Windows allocated a fresh visible console window titled after the CLI executable.Fix
Add
windowsHide: true(maps toCREATE_NO_WINDOW; a no-op on POSIX) to every inner CLI spawn:buildClaudeInvocation/buildTrailingPromptInvocationinsrc/hooks/wiki-worker-spawn.ts(covers the claude/codex/cursor/pi wiki workers)execFileSync(src/hooks/hermes/wiki-worker.ts)src/skillify/gate-runner.ts)Tests
tests/claude-code/wiki-worker-windows.test.ts)tests/hermes/hermes-wiki-worker-source.test.ts,tests/claude-code/skillify-gate-runner.test.ts)tsc --noEmitclean,npm run buildclean, 4415/4415 tests passingFollow-up to #250, which fixed summary generation itself on Windows; this removes the remaining visible-console regression reported there.
Session Context
Session transcripts (investigation + PR creation)
Summary by CodeRabbit
Bug Fixes
Tests